-
-
Notifications
You must be signed in to change notification settings - Fork 884
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
WIP: Email localization (fixes #500) #2053
Conversation
6be867c
to
e17cce7
Compare
Updated, basic implementation is now done. For now it uses separate translation files (which need to get moved into lemmy-translations repo). After upstream changes we should be able to merge it with existing translation files. I'm not really happy with the way the code is structured now, as email notification logic is split across crates |
e17cce7
to
82c3ca3
Compare
82c3ca3
to
61bd53d
Compare
@dessalines Could you have a look at this? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! Let me know when you're ready to take it out of draft and I can help test it a bit.
@@ -104,11 +105,16 @@ impl PerformCrud for CreatePrivateMessage { | |||
LocalUserView::read_person(conn, recipient_id) | |||
}) | |||
.await??; | |||
let lang = get_user_lang(&local_recipient); | |||
let inbox_link = format!("{}/inbox", context.settings().get_protocol_and_hostname()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Was confused at first, but yes this does make sense, its their lemmy-ui inbox page.
&inserted_local_user.email.expect("email was provided"), | ||
&inserted_person.name, | ||
&local_user_view, | ||
&email, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, iirc email is only in LocalUser, not the LocalUserSettings or Safe.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure what you mean, local_user_view contains LocalUser. And I dont think i changed any of the logic.
crates/utils/translations/en.json
Outdated
"notification_mentioned_by_body": "<h1>Person Mention</h1><br><div>{username} - {comment_text}</div><br><a href={inbox_link}>inbox</a>", | ||
"notification_private_message_subject": "Private message from {username}", | ||
"notification_private_message_body": "<h1>Private message</h1><br><div>{username} - {message_text}</div><br><a href={inbox_link}>inbox</a>" | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, this is pretty easy, as long as its positional then.
Not super looking forward to setting up a third weblate for lemmy... but its pry gonna need one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, at least temporarily until the rosetta crate is updated.
} | ||
|
||
pub fn get_user_lang(user: &LocalUserView) -> Lang { | ||
let user_lang = LanguageId::new(user.local_user.lang.clone()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this doesn't work correctly for things like pt-br
, we might have to unfortunately create mappings. But lets not worry about that yet.
61bd53d
to
eb5d255
Compare
1b41054
to
0a7474d
Compare
0a7474d
to
311edcf
Compare
311edcf
to
a32cd77
Compare
a32cd77
to
e554eea
Compare
e554eea
to
53f2399
Compare
I setup weblate translations. |
Needs some clarifications from upstream: baptiste0928/rosetta#5